Changed multiaddr backing from Arc<Vec<u8>> to EcoVec<u8>#89
Changed multiaddr backing from Arc<Vec<u8>> to EcoVec<u8>#89imbrem wants to merge 4 commits intomultiformats:masterfrom
Conversation
|
Thanks @imbrem for investing into rust-multiaddr performance characteristics. Can you please provide a benchmark that proves that this pull request improves performance? |
|
I added some basic benchmarks in the branch https://github.com/imbrem/rust-multiaddr/tree/multiaddr-benches; these indicate a 25% performance improvement in creating and cloning ipv4 addresses, and about the same performance for ipv6 addresses. The biggest performance improvements should be in manipulating ipv4 addresses, but making a benchmark for this is more complicated; I should be able to find some time to do this in a few days. |
I can not think of a place where we manipulate an IPv4 address in a hot-path. |
|
If you want I can try to benchmark memory usage, which should be significantly improved as we eliminate the Arc allocation holding the Vec. Another option is to modify EcoVec to have a longer buffer so it can hold an ipv6 address inline or more; this would leave memory the same as the original (since the larger EcoVec would have a larger storage footprint) but now more multiaddrs would share in the performance improvement. Issue is I’d probably have to fork EcoVec in the latter case since it adds a lot to the complexity. |
You could test with some downstream application using rust-libp2p, e.g. https://github.com/mxinden/kademlia-exporter/. Maybe the change here shows up in heaptrack of a CPU flamegraph. |
While this increases the size of a multiaddr on the stack or in a container (from 1 word to 2 words), it decreases the overall memory usage of the multiaddr (from 2 allocations to 1 allocation) and avoids a double dereference when manipulating multiaddrs. Hence, I believe that this should be a win for most applications.
This PR currently includes a hack so that
EcoVec<u8>implementsWrite, but if typst/ecow#23 lands, it can be removed.